[flyteagent] Add agents and agentForTaskTypes in helm chart
#6239
Conversation
Signed-off-by: mao3267 <[email protected]>
Signed-off-by: mao3267 <[email protected]>
Code Review Agent Run Status
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6239 +/- ##
=======================================
Coverage 36.86% 36.86%
=======================================
Files 1318 1318
Lines 134767 134767
=======================================
+ Hits 49684 49685 +1
+ Misses 80753 80752 -1
Partials 4330 4330
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
This is the updated configmap! agent-service:
defaultAgent:
defaultTimeout: 10s
endpoint: k8s://flyteagent.flyte:8000
insecure: true
timeouts:
GetTask: 10safter agent-service:
agentForTaskTypes:
- noop_task: custom_agent
agents:
custom_agent:
endpoint: k8s://flyte_custom_agent.flyte:8000
insecure: true
defaultAgent:
defaultTimeout: 10s
endpoint: k8s://flyteagent.flyte:8000
insecure: true
timeouts:
CreateTask: 10s
DeleteTask: 10s
ExecuteTaskSync: 10s
GetTask: 10s
supportedTaskTypes: [] |
Signed-off-by: Future-Outlier <[email protected]>
Future-Outlier
changed the title
agents and agentForTaskTypes in helm chart
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Code Review Agent Run #5f1d05Actionable Suggestions - 2
Review Details
AI Code Review powered by |
Changelist by BitoThis pull request implements the following key changes.
|
| GetTask: 10s | ||
| supportedTaskTypes: [] |
There was a problem hiding this comment.
Consider adding task types to the supportedTaskTypes array as an empty array may prevent the agent from handling any tasks. You might want to specify the task types this agent should support.
Code suggestion
Check the AI-generated fix before applying
| supportedTaskTypes: [] | |
| supportedTaskTypes: ["noop_task"] |
Code Review Run #5f1d05
Is this a valid issue, or was it incorrectly flagged by the Agent?
- it was incorrectly flagged
| // Verify agent_1 was called (failed) | ||
| // agent1Client.AssertCalled(t, "CreateTask", mock.Anything, mock.Anything) |
There was a problem hiding this comment.
The commented out assertion agent1Client.AssertCalled(t, "CreateTask", mock.Anything, mock.Anything) should be uncommented since it verifies that the agent client was called correctly. Without this assertion, the test is not fully validating the expected behavior.
Code suggestion
Check the AI-generated fix before applying
| // Verify agent_1 was called (failed) | |
| // agent1Client.AssertCalled(t, "CreateTask", mock.Anything, mock.Anything) | |
| agent1Client.AssertCalled(t, "CreateTask", mock.Anything, mock.Anything) |
Code Review Run #5f1d05
Is this a valid issue, or was it incorrectly flagged by the Agent?
- it was incorrectly flagged
Tracking issue
#3936
Why are the changes needed?
We aim to enhance the default configuration by adding more customization options for agent users. Specifically, we are introducing custom agent deployments in
AgentDeploymentsand defining task types for specific agents inAgentForTaskTypes. These additions enable greater flexibility across various scenarios.What changes were proposed in this pull request?
How was this patch tested?
Labels
added
Setup process
Screenshots
Check all the applicable boxes
Related PRs
#6179
Docs link
None
Summary by Bito
This PR enhances Flyte agent configuration by introducing custom agent deployments and task type mapping in Helm charts. The implementation adds timeouts for agent operations, configures custom agent endpoints, and establishes task type to agent mappings. The changes improve flexibility and customization options while including comprehensive test coverage.Unit tests added: True
Estimated effort to review (1-5, lower is better): 5